Overall Assessment

⚠️ Needs changes

This is a docs-only PR, but the new practitioner decision tree and walkthrough make methodology-bearing estimator-selection and interpretation claims. I found two unmitigated P1 issues in that guidance, plus one broken survey example. I did not find matching REGISTRY.md deviation notes or TODO.md tracking that would mitigate them.

Executive Summary

• Severity: P1. The new ContinuousDiD section presents dose-response / ATT(d) interpretation without the Strong Parallel Trends caveat required by the Methodology Registry.
• Severity: P1. The new SyntheticDiD example describes treatment starting at period 7 but omits post_periods; the estimator will instead use its midpoint default and analyze a different post window.
• Severity: P2. The new SurveyDesign example uses unsupported data= and weight= kwargs and will fail as written.

Methodology

• Severity: P1. The ContinuousDiD guidance in docs/practitioner_decision_tree.rst:141 says the method gives a dose-response curve and that ATT(d) is “the lift at each spending level,” but the registry says plain PT identifies only ATT(d|d) / ATT^loc, while ATT(d) and ACRT(d) require Strong Parallel Trends docs/methodology/REGISTRY.md:465. The existing estimator guide already carries that caveat docs/choosing_estimator.rst:252. Impact: practitioners can over-interpret overall_att and the dose-response output as point-identified under weaker assumptions than the method requires. Concrete fix: add a short practitioner-facing note here that full dose-response interpretation requires SPT, plus the untreated-group / balanced-panel / time-invariant-dose conditions, or soften the text to the PT-identified quantity instead.
• Severity: P1. The SyntheticDiD example in docs/practitioner_decision_tree.rst:183 narrates treatment starting at period 7, but SyntheticDiD.fit defaults post_periods to the last half of observed periods when that argument is omitted diff_diff/synthetic_did.py:190 diff_diff/synthetic_did.py:296. With 12 periods, that makes period 6 part of the post window. Impact: the example estimates a different estimand than the text describes and can silently report the wrong ATT. Concrete fix: pass the actual post periods explicitly, for example from the generated post column, or change the DGP so the midpoint default matches the narrative.

Code Quality

No findings in scope for a docs-only PR.

Performance

No findings in scope for a docs-only PR.

Maintainability

No findings. The docs/index.rst and docs/doc-deps.yaml additions look internally consistent.

Tech Debt

No separate deferrable tech-debt item. The blocking issues above are not mitigated by existing TODO.md entries.

Security

No findings; I did not see secrets, credentials, or PII introduced by the diff.

Documentation/Tests

• Severity: P2. The survey snippet in docs/practitioner_decision_tree.rst:226 uses SurveyDesign(data=..., weight=...), but the actual dataclass begins at diff_diff/survey.py:27 and takes weights, strata, psu, fpc, etc.; it does not accept data or weight. Impact: the example raises TypeError as written, so the PR’s “code blocks verified executable” claim does not hold for this snippet. Concrete fix: change it to SurveyDesign(weights="sample_weight", strata="stratum", psu="cluster_id") (and fpc= if intended), then rerun snippet validation.
• Severity: P3. docs/practitioner_getting_started.rst:120 says print(results) prints a summary table, but DiDResults.__repr__ is a one-line summary and the full table comes from summary() diff_diff/results.py:99. Impact: minor user confusion. Concrete fix: change the snippet to print(results.summary()) or adjust the prose.

Path to Approval

1. Add an explicit ContinuousDiD note in docs/practitioner_decision_tree.rst stating that dose-response / ATT(d) interpretation requires Strong Parallel Trends, and mention the untreated-group, balanced-panel, and time-invariant-dose requirements.
2. Fix the SyntheticDiD example so its post_periods matches the stated treatment start, either by passing the actual post-period list or by changing the generated DGP to align with the midpoint default.
3. Correct the survey example to the real SurveyDesign API and rerun doc-snippet validation on the new .rst files.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: e98506a70cc77be2f0aa422d07e3367757ec9395

Overall Assessment

✅ Looks good

The prior blocking findings are resolved, and I did not find any unmitigated P0/P1 issues in the changed files.

Executive Summary

• The prior ContinuousDiD methodology issue is fixed: the practitioner decision tree now states that ATT(d) / ACRT(d) require Strong Parallel Trends and that standard PT identifies only the binarized effect, with the untreated-group / balanced-panel / time-invariant-dose requirements surfaced in docs/practitioner_decision_tree.rst:160 and docs/practitioner_decision_tree.rst:168.
• The prior SyntheticDiD example issue is fixed: the few-markets example now derives and passes explicit post_periods in docs/practitioner_decision_tree.rst:201, matching the estimator’s documented default behavior in diff_diff/synthetic_did.py:50 and diff_diff/synthetic_did.py:297.
• The prior survey example API issue is fixed: the new snippet uses SurveyDesign(weights=..., strata=..., psu=...) in docs/practitioner_decision_tree.rst:241, consistent with diff_diff/survey.py:37.
• The prior results-printing issue is fixed: the getting-started guide now uses print(results.summary()) in docs/practitioner_getting_started.rst:120, matching diff_diff/results.py:99.
• Residual issues are non-blocking: one P2 docs-example completeness issue and one P3 methodology caveat that is already documented in the registry.

Methodology

• Severity: P3 informational. Impact: The survey guidance in docs/practitioner_decision_tree.rst:229 and docs/practitioner_getting_started.rst:293 says to combine any estimator with SurveyDesign and implies the correction is automatic. For SyntheticDiD, full-design surveys with strata/PSU/FPC require variance_method="bootstrap"; the default variance_method="placebo" supports weights-only, per diff_diff/synthetic_did.py:50, diff_diff/synthetic_did.py:218, diff_diff/synthetic_did.py:272, docs/choosing_estimator.rst:700, and the documented note in docs/methodology/REGISTRY.md:1298. Because that caveat is already documented in the registry, I’m treating this as informational rather than blocking. Concrete fix: add a one-sentence SyntheticDiD caveat in both practitioner survey sections or link directly to the survey compatibility matrix.

Code Quality

• No findings in scope for a docs-only PR.

Performance

• No findings in scope for a docs-only PR.

Maintainability

• No findings. The new navigation and doc-dependency wiring look internally consistent across docs/index.rst:47 and docs/doc-deps.yaml:104.

Tech Debt

• No new tech-debt finding. The only residual methodology caveat above is already documented in the registry, so it is not a blocker and does not need a new TODO.md entry.

Security

• No findings; I did not see secrets, credentials, or PII introduced by the changed files.

Documentation/Tests

• Severity: P2. Impact: The survey snippet in docs/practitioner_decision_tree.rst:236 is not self-contained: it calls did.fit(data, ...) without defining data in that branch. Because this page is organized as alternative scenario branches, a reader who jumps directly to “Survey Data” can copy a block that fails immediately with NameError. Concrete fix: either add a minimal data = ... setup to that snippet or explicitly label it as partial and assuming an already-prepared data DataFrame.